Skip to content

lib: remove optional builtins prefixes from prelude functions#447402

Merged
philiptaron merged 2 commits into
NixOS:masterfrom
trueNAHO:lib-remove-optional-builtins-prefixes-from-prelude-functions
Sep 30, 2025
Merged

lib: remove optional builtins prefixes from prelude functions#447402
philiptaron merged 2 commits into
NixOS:masterfrom
trueNAHO:lib-remove-optional-builtins-prefixes-from-prelude-functions

Conversation

@trueNAHO

@trueNAHO trueNAHO commented Sep 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Member 
commit 6177c4ad724f155e05c15e686cc9f27eecd3682b
Author: NAHO <90870942+trueNAHO@users.noreply.github.com>
Date:   2025-09-30 08:48:46 +0200

    lib: remove optional builtins prefixes from prelude functions

    Remove optional builtins prefixes from prelude functions by running:

        builtins=(
          abort
          baseNameOf
          break
          derivation
          derivationStrict
          dirOf
          false
          fetchGit
          fetchMercurial
          fetchTarball
          fetchTree
          fromTOML
          import
          isNull
          map
          null
          placeholder
          removeAttrs
          scopedImport
          throw
          toString
          true
        )

        fd --type file . lib --exec-batch sed --in-place --regexp-extended "
          s/\<builtins\.($(
            printf '%s\n' "${builtins[@]}" |
              paste --delimiter '|' --serial -
          ))\>/\1/g
        "

        nix fmt

commit e8f998c3bc46f0948457267cc47ddcbbb2145462
Author: NAHO <90870942+trueNAHO@users.noreply.github.com>
Date:   2025-09-30 09:36:35 +0200

    lib: add builtins prefixes for prelude functions to improve clarity

Related PRs intentionally split up to simplify review and merging:

  1. doc: remove optional builtins prefixes from prelude functions #447401
  2. lib: remove optional builtins prefixes from prelude functions #447402
  3. pkgs: remove optional builtins prefixes from prelude functions #447404
  4. nixos: remove optional builtins prefixes from prelude functions #447403
  5. treewide: remove optional builtins prefixes from prelude functions #444432

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@trueNAHO
lib: remove optional builtins prefixes from prelude functions
6177c4a 
Remove optional builtins prefixes from prelude functions by running:

    builtins=(
      abort
      baseNameOf
      break
      derivation
      derivationStrict
      dirOf
      false
      fetchGit
      fetchMercurial
      fetchTarball
      fetchTree
      fromTOML
      import
      isNull
      map
      null
      placeholder
      removeAttrs
      scopedImport
      throw
      toString
      true
    )

    fd --type file . lib --exec-batch sed --in-place --regexp-extended "
      s/\<builtins\.($(
        printf '%s\n' "${builtins[@]}" |
          paste --delimiter '|' --serial -
      ))\>/\1/g
    "

    nix fmt
@trueNAHO trueNAHO mentioned this pull request Sep 30, 2025
Merged
13 tasks
@qweered

qweered commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

I am also for reverting documentation and comments changes

This was referenced Sep 30, 2025
Merged
Merged
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 6.topic: module system About "NixOS" module system internals 6.topic: lib The Nixpkgs function library labels Sep 30, 2025
@trueNAHO trueNAHO mentioned this pull request Sep 30, 2025
Merged
13 tasks
@nixpkgs-ci nixpkgs-ci Bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Sep 30, 2025
qweered
qweered reviewed Sep 30, 2025
View reviewed changes
Comment thread lib/meta.nix
@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 1 This PR was reviewed and approved by one person. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Sep 30, 2025
philiptaron
philiptaron approved these changes Sep 30, 2025
View reviewed changes

@philiptaron philiptaron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the lib tests1 on this PR and they pass (which makes sense.)

Footnotes

  1. nix-build lib/tests/release.nix ↩

Comment thread lib/trivial.nix
@philiptaron philiptaron added this pull request to the merge queue Sep 30, 2025
Merged via the queue into NixOS:master with commit 4499889 Sep 30, 2025
31 of 33 checks passed
@MattSturgeon

MattSturgeon commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

I ran the lib tests1 on this PR and they pass

Q: does the release test still cover various implementations of nix, e.g. lix & snix? I know historically they haven't had full parity with builtins, e.g. lix doesn't (didn't?) have builtins.warn. This makes me wonder if they have full parity with the prelude.

@trueNAHO

trueNAHO commented Sep 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Q: does the release test still cover various implementations of nix, e.g. lix & snix? I know historically they haven't had full parity with builtins, e.g. lix doesn't (didn't?) have builtins.warn.

IIUC, https://gerrit.lix.systems/c/lix/+/2248 is supposed to, or will, fix this.

This makes me wonder if they have full parity with the prelude.

Good point. I suggest halting merging of the remaining PRs until we know the answer to this, although they can still be reviewed. I marked them as draft to indicate this.

Since I am unfamiliar with Lix, pinging the following people hopefully brings more qualified people into the discussion: @lf-, @Qyriad.

Seems like this is not a totally "safe" PR after all:

I've been burned merging "safe" PRs in the nixos/ directory several times before. If you split the PR as asked, it will reduce the scope and I will be able to merge those.

-- #444432 (comment)

Hopefully, my lack of experience and foresight with such large-scale projects has not caused any trouble. If it did, feel free to revert #447401 and #447402 ASAP.

@philiptaron

philiptaron commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

No need to worry. It's all working appropriately.

#!/usr/bin/env bash
set -euo pipefail
s="[abort baseNameOf break derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true]"

for v in lixPackageSets.lix_2_93.lix nixVersions.nix_2_28 nixVersions.nix_2_30 nixVersions.nix_2_31; do
  "$(nix-build -A $v --no-out-link)/bin/nix-instantiate" --strict --eval --expr "$s"
done

Returns:

[ <PRIMOP> <PRIMOP> <PRIMOP> <LAMBDA> <PRIMOP> <PRIMOP> false <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> null <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> true ]
[ <PRIMOP> <PRIMOP> <PRIMOP> <LAMBDA> <PRIMOP> <PRIMOP> false <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> null <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> true ]
[ <PRIMOP> <PRIMOP> <PRIMOP> <LAMBDA> <PRIMOP> <PRIMOP> false <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> null <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> true ]
[ <PRIMOP> <PRIMOP> <PRIMOP> <LAMBDA> <PRIMOP> <PRIMOP> false <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> null <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> <PRIMOP> true ]

Snix is not yet published in Nixpkgs at all. It's missing the following:

  1. break
  2. fetchMercurial
  3. fetchTree
  4. scopedImport

We can ignore that. Snix will catch up, and these aren't core functions.

@trueNAHO trueNAHO mentioned this pull request Oct 6, 2025
Open
13 tasks
trueNAHO added a commit to trueNAHO/nixpkgs that referenced this pull request Oct 6, 2025
@trueNAHO
CONTRIBUTING: establish builtins prefix convention for prelude
abc82de 
Establish the builtins prefix convention for prelude attributes to
progress [1] ("Unwritten style guides should be written"), following the
treewide removal of the builtins prefix for prelude attributes [2] [3]
[4] [5] [6].

[1]: NixOS#387072
[2]: NixOS#444432
[3]: NixOS#447401
[4]: NixOS#447402
[5]: NixOS#447403
[6]: NixOS#447404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philiptaron philiptaron philiptaron approved these changes
+1 more reviewer
@qweered qweered qweered approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: lib The Nixpkgs function library 6.topic: module system About "NixOS" module system internals 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@trueNAHO @qweered @MattSturgeon @philiptaron